AEP: extending the REST API to support mutating (POST) queries #24
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chrisjsewell
changed the title
|
thanks @NinadBhat - I'll go through the PR tomorrow |
ltalirz
reviewed
May 5, 2021
There was a problem hiding this comment.
thanks a lot @NinadBhat and sorry for the delay.
this looks good as a first draft, here comes my first review
006_extend_restapi/readme.md
Outdated
| |------------|------------------------------------------------------------------| | ||
| | Title | Extending the AiiDA REST API towards workflow management | | ||
| | Authors | [Ninad Bhat](mailto:[email protected]) (NinadBhat) | | ||
| | Champions | [Leopold Talirz](mailto:[email protected]) (Italirz) | |
There was a problem hiding this comment.
Suggested change
| | Champions | [Leopold Talirz](mailto:[email protected]) (Italirz) | | |
| | Champions | [Leopold Talirz](mailto:[email protected]) (ltalirz) | |
006_extend_restapi/readme.md
Outdated
|
|
||
| The Proposal is to make three additions to AiiDA REST API. | ||
|
|
||
| 1. Add JSON schema validation and implementing authorisation |
There was a problem hiding this comment.
Suggested change
| 1. Add JSON schema validation and implementing authorisation | |
| 1. Add validation of the QueryBuilder JSON using JSON schema | |
| 1. Add authentication/authorisation |
006_extend_restapi/readme.md
Outdated
Comment on lines
22
to
23
| 2. Add POST methods to /users, /computers, /nodes and /groups endpoint | ||
| 3. Creating a new /processes endpoint |
There was a problem hiding this comment.
Suggested change
| 2. Add POST methods to /users, /computers, /nodes and /groups endpoint | |
| 3. Creating a new /processes endpoint | |
| 1. Add POST methods to /users, /computers, /nodes and /groups endpoints | |
| 1. Add a new /processes endpoint |
006_extend_restapi/readme.md
Outdated
| ## Detailed Explanation | ||
|
|
||
| ### Schema Validation and Authorisation | ||
| Currently, only QueryBuilder endpoint has a post method. Post endpoint for QueryBuilder was implemented in [PR #4337](https://github.com/aiidateam/aiida-core/pull/4337). The current implementation only checks if the posted JSON is a dict at [L269](https://github.com/aiidateam/aiida-core/blob/develop/aiida/restapi/resources.py#L269). |
There was a problem hiding this comment.
Suggested change
| Currently, only QueryBuilder endpoint has a post method. Post endpoint for QueryBuilder was implemented in [PR #4337](https://github.com/aiidateam/aiida-core/pull/4337). The current implementation only checks if the posted JSON is a dict at [L269](https://github.com/aiidateam/aiida-core/blob/develop/aiida/restapi/resources.py#L269). | |
| Currently, only the `/querybuilder` endpoint accepts `POST` requests (implemented in [PR #4337](https://github.com/aiidateam/aiida-core/pull/4337)). | |
| The current implementation only checks if the posted JSON is a `dict` (see [L269](https://github.com/aiidateam/aiida-core/blob/develop/aiida/restapi/resources.py#L269)). |
006_extend_restapi/readme.md
Outdated
|
|
||
| ## Detailed Explanation | ||
|
|
||
| ### Schema Validation and Authorisation |
There was a problem hiding this comment.
Let's split this into two since they are really different aspects and can be addressed independently
|
|
||
| ``` | ||
|
|
||
| { |
There was a problem hiding this comment.
Most of this information already exists in the translators
https://github.com/aiidateam/aiida-core/blob/5f50f0335761386ddacbb5de4a6934b659b20e2d/aiida/restapi/translator/user.py#L40-L69
We should reuse this, i.e. either generate the JSON schema from the projectable_properties or vice versa (to think about).
Furthermore, the JSON schema should contain the "required" field with information on whether each of the respective fields
| ``` | ||
|
|
||
| { | ||
| "first_name": { |
There was a problem hiding this comment.
You might want/need to add some extra metadata around
I would have thought more something like
{
"$id": "https://raw.githubusercontent.com/aiidateam/aiida-core/master/aiida/restapi/schemas/user.schema.json",
"$schema": "https://json-schema.org/draft/2020-12/schema",
"title": "User",
"type": "object",
"properties": {
"first_name": {
"type": "string",
"description": "The person's first name."
}
...
}
006_extend_restapi/readme.md
Outdated
| }, | ||
| "extras": { | ||
| "description": "" | ||
| "type": "json object" |
There was a problem hiding this comment.
Suggested change
| "type": "json object" | |
| "type": "object" |
006_extend_restapi/readme.md
Outdated
| } | ||
| "attributes": { | ||
| "description": "attributes of the node" | ||
| "type": "json object" |
There was a problem hiding this comment.
Suggested change
| "type": "json object" | |
| "type": "object" |
006_extend_restapi/readme.md
Outdated
|
|
||
| ### Cons | ||
| 1. Development time spent on extending API | ||
| 2. Also the time spent on maintaining API |
There was a problem hiding this comment.
Suggested change
| 2. Also the time spent on maintaining API | |
| 2. Increased maintenance effort for the REST API for changes in the ORM | |
| The second point could be minimized by aiming for a single source for the attributes of the ORM entities that is used both by the python API and the REST API. |
|
A few extra things to add:
|
sphuber
changed the title
|
Prototype implementation in https://github.com/aiidateam/aiida-restapi |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Have kept the status as "draft".
submittedREADME.md